From c141fe906facdfb8f1ac1a312c4fa38b140a890b Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 30 Jan 2025 14:31:01 +0100 Subject: [PATCH] set the parent folder read/write always when downloading a new file that would also cover code paths for virtual files Signed-off-by: Matthieu Gallien --- src/libsync/propagatedownload.cpp | 78 ++++++++++++++++++++----------- src/libsync/propagatedownload.h | 5 ++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 36f1251db..9c6f1b579 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -490,7 +490,8 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked() // For virtual files just dehydrate or create the file and be done if (_item->_type == ItemTypeVirtualFileDehydration) { - QString fsPath = propagator()->fullLocalPath(_item->_file); + const auto fsPath = propagator()->fullLocalPath(_item->_file); + makeParentFolderModifiable(fsPath); if (!FileSystem::verifyFileUnchanged(fsPath, _item->_previousSize, _item->_previousModtime)) { propagator()->_anotherSyncNeeded = true; done(SyncFileItem::SoftError, tr("File has changed since discovery"), ErrorCategory::GenericError); @@ -499,6 +500,7 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked() qCDebug(lcPropagateDownload) << "dehydrating file" << _item->_file; if (FileSystem::isLnkFile(fsPath)) { + makeParentFolderModifiable(_tmpFile.fileName()); const auto convertResult = vfs->convertToPlaceholder(fsPath, *_item); if (!convertResult) { qCCritical(lcPropagateDownload()) << "error when converting a shortcut file to placeholder" << convertResult.error(); @@ -532,6 +534,10 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked() } if (_item->_type == ItemTypeVirtualFile && !propagator()->localFileNameClash(_item->_file)) { qCDebug(lcPropagateDownload) << "creating virtual file" << _item->_file; + + const auto fsPath = propagator()->fullLocalPath(_item->_file); + makeParentFolderModifiable(fsPath); + // do a klaas' case clash check. if (propagator()->localFileNameClash(_item->_file)) { done(SyncFileItem::FileNameClash, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file)), ErrorCategory::GenericError); @@ -666,6 +672,7 @@ void PropagateDownloadFile::startDownload() tmpFileName = createDownloadTmpFileName(_item->_file); } _tmpFile.setFileName(propagator()->fullLocalPath(tmpFileName)); + makeParentFolderModifiable(_tmpFile.fileName()); _resumeStart = _tmpFile.size(); if (_resumeStart > 0 && _resumeStart == _item->_size) { @@ -680,32 +687,6 @@ void PropagateDownloadFile::startDownload() FileSystem::setFileReadOnly(_tmpFile.fileName(), false); } -#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 - try { - const auto newDirPath = std::filesystem::path{_tmpFile.fileName().toStdWString()}; - Q_ASSERT(newDirPath.has_parent_path()); - _parentPath = newDirPath.parent_path(); - } - catch (const std::filesystem::filesystem_error &e) - { - qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str(); - } - catch (const std::system_error &e) - { - qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what(); - } - catch (...) - { - qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights"; - } - - if (FileSystem::isFolderReadOnly(_parentPath)) { - FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite); - emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); - _needParentFolderRestorePermissions = true; - } -#endif - if (!_tmpFile.open(QIODevice::Append | QIODevice::Unbuffered)) { qCWarning(lcPropagateDownload) << "could not open temporary file" << _tmpFile.fileName(); done(SyncFileItem::NormalError, _tmpFile.errorString(), ErrorCategory::GenericError); @@ -786,6 +767,47 @@ void PropagateDownloadFile::setDeleteExistingFolder(bool enabled) _deleteExisting = enabled; } +void PropagateDownloadFile::done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category) +{ +#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 + if (_needParentFolderRestorePermissions) { + FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly); + emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); + _needParentFolderRestorePermissions = false; + } +#endif + PropagateItemJob::done(status, errorString, category); +} + +void PropagateDownloadFile::makeParentFolderModifiable(const QString &fileName) +{ +#if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 + try { + const auto newDirPath = std::filesystem::path{fileName.toStdWString()}; + Q_ASSERT(newDirPath.has_parent_path()); + _parentPath = newDirPath.parent_path(); + } + catch (const std::filesystem::filesystem_error &e) + { + qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str(); + } + catch (const std::system_error &e) + { + qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what(); + } + catch (...) + { + qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights"; + } + + if (FileSystem::isFolderReadOnly(_parentPath)) { + FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite); + emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); + _needParentFolderRestorePermissions = true; + } +#endif +} + const char owncloudCustomSoftErrorStringC[] = "owncloud-custom-soft-error-string"; void PropagateDownloadFile::slotGetFinished() { @@ -1329,7 +1351,7 @@ void PropagateDownloadFile::downloadFinished() #if !defined(Q_OS_MACOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_15 if (_needParentFolderRestorePermissions) { - FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadWrite); + FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly); emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring())); _needParentFolderRestorePermissions = false; } diff --git a/src/libsync/propagatedownload.h b/src/libsync/propagatedownload.h index 87aec08dc..912cab03e 100644 --- a/src/libsync/propagatedownload.h +++ b/src/libsync/propagatedownload.h @@ -220,6 +220,11 @@ public: */ void setDeleteExistingFolder(bool enabled); +protected: + void done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category) override; + + void makeParentFolderModifiable(const QString &fileName); + private slots: /// Called when ComputeChecksum on the local file finishes, /// maybe the local and remote checksums are identical? -- 2.30.2